-
Notifications
You must be signed in to change notification settings - Fork 422
0.1 bindings #3545
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
0.1 bindings #3545
Conversation
...as the bindings generation does not currently have the ability to map a reference to a `NodeId` inside a tuple.
Bindings can't handle references in return types, so reduce the visibility to pub(crate).
The bindings currently get confused by the implicit `Sign` type, so we temporarily remove it on the `impl` here.
Re-exports in Rust make `use` statements a little shorter, but for otherwise don't materially change a crate's API. Sadly, the C bindings generator currently can't figure out re-exports, but it also exports everything into one global namespace, so it doesn't matter much anyway.
Having struct fields with references to other structs is tough in our bindings logic, but even worse if the fields are in an enum. Its simplest to just take the clone penalty here.
The scorer currently relies on an associated type for the fee parameters. This isn't supportable at all in bindings, and for lack of a better option we simply hard-code the parameter for all scorers to `ProbabilisticScoringFeeParameters`.
These are not expressible in C/most languages, and thus must be hidden.
Rather than `ChannelMonitor` only being clonable when the signer is clonable, we require all signers to be clonable and then make all `ChannelMonitor`s clonable.
As we cannot express slices without inner references in bindings wrappers.
Bindings don't accept dyn traits, but instead map any traits to a single dynamic struct. Thus, we can always take a specific trait to accept any implementation, which we do here.
Mapping an `Hmac<Sha256>` would require somewhat custom logic as we'd have to behave differently based on generic parameters, so its simplest to just swap it to a `[u8; 32]` instead.
The bindings really should support this, but currently they don't and its late enough in the release cycle I don't want to try to fix that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given we're not exposing the lightning-transaction-sync crate in bindings and all the modules should be hidden behind features anyways, why are these changes necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, they were just in the changeset cause we'd started trying to expose tx-sync. Might as well leave them so that we can eventually-maybe expose it.
7e2c5ef
into
lightningdevkit:0.1-bindings
Basically just a rebase of the existing stuff, but it seems to work.